Conversation
| options = defaultValue; | ||
| defaultValue = undefined; | ||
| } | ||
| let missing = false; |
There was a problem hiding this comment.
This hunk of code here tests if the string passed in for processing is a number in various ways. We're using the numeral package here, so just let it tell us. Also, the real bug was using isNaN on the numeralValue object, which was erroring. Not sure why this took so long to fail / be found.
There was a problem hiding this comment.
I'm sure it's the only unfound bug in the codebase!
| "/kb/dev_container/narrative/src/config.json.templ:/kb/dev_container/narrative/kbase-extension/static/kbase/config/config.json" \ | ||
| kbase-narrative --no-browser --port=$JUPYTER_PORT 2>&1 | tee $OUTPUT_FILE & | ||
| bg_pid=$! | ||
| bg_pid=$(jobs -p) |
There was a problem hiding this comment.
Tests ongoing - I want to see how it behaves on GHA.
But when running on my local MacOS, $bg_pid had the pid of the tee process, not kbase-narrative. Running jobs -p fixes that.
| # } | ||
|
|
||
| # echo "Killing process $bg_pid and children" | ||
| pkill -P "$bg_pid" |
There was a problem hiding this comment.
We're already using pgrep in kill_descendant_processes, so pkill should be available and basically does the same thing. I think.
| } | ||
| ] | ||
| }, | ||
| { |
There was a problem hiding this comment.
These tests seemed to be effectively redundant with slightly different data. They may be relevant, and just generated by different apps, but as far as the viewer's concerned, they seem to be similar enough.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3652 +/- ##
===========================================
- Coverage 25.92% 25.88% -0.04%
===========================================
Files 461 461
Lines 46652 46652
===========================================
- Hits 12093 12078 -15
- Misses 34559 34574 +15 see 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
2 participants
STILL IN PROGRESS - DO NOT MERGE YET
Description of PR purpose/changes
Fix up the broken integration tests. They've failed due to a combination of out-of-date dependencies, a bug or two, and inaccessible test narratives.
I've changed relevant narratives to be accessible (or at least viewable) by the
narrative_testCI account. So they should all work ok with anarrative_testauth token, which should be a GH secret.Here's the changes:
narrative_testnarrative_testaccount (TODO: remake these undernarrative_test)Jira Ticket / Issue
Related Jira ticket: https://kbase-jira.atlassian.net/browse/UIP-52
DATAUP-69 Adds a PR template)Testing Instructions
Dev Checklist:
formatandcheckon changed Python code manually or with a git precommit hookUpdating Version and Release Notes (if applicable)